Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix pivot basic rewrite with new PyZX version #334

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

colltoaction
Copy link
Contributor

This draft uncovers the issue with pivoting which I will solve next. cc @RazinShaikh. #256

Traceback (most recent call last):
  File "/home/mcoll/repos/github.com/colltoaction/zxlive/zxlive/rewrite_action.py", line 281, in do_rewrite
    node.rewrite_action.do_rewrite(self.proof_panel)
  File "/home/mcoll/repos/github.com/colltoaction/zxlive/zxlive/rewrite_action.py", line 114, in do_rewrite
    g, rem_verts = self.apply_rewrite(g, matches)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/mcoll/repos/github.com/colltoaction/zxlive/zxlive/rewrite_action.py", line 129, in apply_rewrite
    etab, rem_verts, rem_edges, check_isolated_vertices = self.rule(g, matches)
                                                          ^^^^^^^^^^^^^^^^^^^^^
  File "/home/mcoll/repos/github.com/colltoaction/zxlive/zxlive/rewrite_data.py", line 117, in rule
    simplification(g)
  File "/home/mcoll/.local/lib/python3.11/site-packages/pyzx/simplify.py", line 109, in pivot_simp
    return simp(g, 'pivot_simp', match_pivot_parallel, pivot, matchf=matchf, quiet=quiet, stats=stats)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/mcoll/.local/lib/python3.11/site-packages/pyzx/simplify.py", line 98, in simp
    g.remove_edges(rem_edges)
  File "/home/mcoll/.local/lib/python3.11/site-packages/pyzx/graph/multigraph.py", line 237, in remove_edges
    self.remove_edge(e)
  File "/home/mcoll/.local/lib/python3.11/site-packages/pyzx/graph/multigraph.py", line 240, in remove_edge
    s,t,ty = edge
    ^^^^^^
ValueError: not enough values to unpack (expected 3, got 2)

@colltoaction
Copy link
Contributor Author

@jvdwetering
Copy link
Collaborator

@colltoaction Do you expect you will continue to work on this? Otherwise I'm gonna close it.

Traceback (most recent call last):
  File "/home/mcoll/repos/github.com/colltoaction/zxlive/zxlive/rewrite_action.py", line 281, in do_rewrite
    node.rewrite_action.do_rewrite(self.proof_panel)
  File "/home/mcoll/repos/github.com/colltoaction/zxlive/zxlive/rewrite_action.py", line 114, in do_rewrite
    g, rem_verts = self.apply_rewrite(g, matches)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/mcoll/repos/github.com/colltoaction/zxlive/zxlive/rewrite_action.py", line 129, in apply_rewrite
    etab, rem_verts, rem_edges, check_isolated_vertices = self.rule(g, matches)
                                                          ^^^^^^^^^^^^^^^^^^^^^
  File "/home/mcoll/repos/github.com/colltoaction/zxlive/zxlive/rewrite_data.py", line 117, in rule
    simplification(g)
  File "/home/mcoll/.local/lib/python3.11/site-packages/pyzx/simplify.py", line 109, in pivot_simp
    return simp(g, 'pivot_simp', match_pivot_parallel, pivot, matchf=matchf, quiet=quiet, stats=stats)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/mcoll/.local/lib/python3.11/site-packages/pyzx/simplify.py", line 98, in simp
    g.remove_edges(rem_edges)
  File "/home/mcoll/.local/lib/python3.11/site-packages/pyzx/graph/multigraph.py", line 237, in remove_edges
    self.remove_edge(e)
  File "/home/mcoll/.local/lib/python3.11/site-packages/pyzx/graph/multigraph.py", line 240, in remove_edge
    s,t,ty = edge
    ^^^^^^
ValueError: not enough values to unpack (expected 3, got 2)
@colltoaction
Copy link
Contributor Author

Hi @jvdwetering! I just took another look at the issue. I changed the rule so that it uses simplify.pivot_simp with auto_simplify_parallel_edges=True and I got the following result:

image

Does it look in accordance to #256 (comment) to you?

@jvdwetering
Copy link
Collaborator

What was the graph that you started with? That one is not semantically equivalent to #256 (comment)
Also, there were some errors reported with the auto-simplify, so that could be going on here as well: zxcalc/pyzx#259

@colltoaction
Copy link
Contributor Author

  1. Open repro file repro-256.zxg.json
  2. Start derivation
  3. Select top two nodes
  4. Click Graph-like rules / pivot

@jvdwetering
Copy link
Collaborator

With the current version of ZXLive I get:
image
Which is semantically correct, it is just that the bottom two wires should dissapear.
The version you get is semantically wrong, though given the changes you made to the code I don't understand why it is wrong.

@jvdwetering
Copy link
Collaborator

In your current implementation with pivot_simp it would also keep doing pivots on the whole graph, ignoring what you selected.
It looks like in your diagram it might have applied the pivot twice, though it still looks wrong to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants